8380391: Update java.smartcardio finalizer to use Cleaner#30683
8380391: Update java.smartcardio finalizer to use Cleaner#30683bchristi-git wants to merge 2 commits intoopenjdk:masterfrom
Conversation
|
👋 Welcome back bchristi! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@bchristi-git The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
| } catch (PCSCException e) { | ||
| context.state = State.REMOVED; | ||
| // Cleaner no longer needs to track | ||
| cleanable.clean(); // FIXME: why? |
There was a problem hiding this comment.
Is this FIXME still open?
Though it seems to be the same behavior as before; there it also set state = State.REMOVED, making finalize() effectively nop.
So is this mostly a "why was this done like this before"?
This is a pull request to convert the finalizer in
sun.security.smartcardio.CardImplto use Cleaner instead. The relevant state is refactored into a Context object, in the standard fashion.This change uses the recommended
try/finally/reachabilityFence()technique to prevent races between the program thread and the Cleaner thread, per theReference.reachabilityFence()API Note:See Reference.reachabilityFence() and java.lang.ref - Memory Visibility for details / background info.
The test creates a
Cardobject, and allows it to be collected. This confirms that the new cleaning action does not hold onto the Owner ("this") object, per the Cleaner class API Note ("it is important that the object implementing the cleaning action does not hold references to the object").I've tried to make the test similar to nearby JavaCard tests. I tried it on Windows using the latest JavaCard Simulator.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/30683/head:pull/30683$ git checkout pull/30683Update a local copy of the PR:
$ git checkout pull/30683$ git pull https://git.openjdk.org/jdk.git pull/30683/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 30683View PR using the GUI difftool:
$ git pr show -t 30683Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/30683.diff
Using Webrev
Link to Webrev Comment